Skip to content

Enable password manager autofill for admin tokens - #873

Merged
Wibias merged 2 commits into
lidge-jun:devfrom
mincia1110:agent/admin-token-password-manager
Aug 3, 2026
Merged

Enable password manager autofill for admin tokens#873
Wibias merged 2 commits into
lidge-jun:devfrom
mincia1110:agent/admin-token-password-manager

Conversation

@mincia1110

@mincia1110 mincia1110 commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Summary

  • replace the remote dashboard's window.prompt() admin-token flow with a standard sign-in form that password managers can recognize
  • keep the submitted admin token in memory only; OpenCodex does not write it to localStorage or sessionStorage
  • preserve the shared 401 resolution gate, cancellation behavior, and focus restoration
  • add localized dialog copy, focused regression coverage, and dashboard documentation

Why

Remote dashboards must authenticate with the raw admin token, but window.prompt() does not expose the standard form and autocomplete semantics used by browser password managers. This forces operators to copy and paste the credential again after reloads instead of letting their chosen password manager offer save and autofill behavior.

The root cause is the prompt primitive itself: it cannot provide stable username/password fields, field names, or autocomplete="current-password" metadata.

Security and privacy

  • token persistence remains entirely browser/password-manager owned and opt-in
  • OpenCodex continues to retain the token only in module memory
  • no token, account identifier, hostname, or operator-specific value is added to source or tests
  • bun run privacy:scan passes

This PR touches management-credential handling and therefore requests explicit security review.

Validation

  • bun run test — 6,867 passed, 8 skipped
  • bun run typecheck
  • bun run privacy:scan
  • cd gui && bun test tests — 492 passed
  • cd gui && bun run lint
  • cd gui && bun run lint:i18n
  • cd gui && bun run build
  • cd docs-site && bun run build

Summary by CodeRabbit

  • New Features

    • Added a localized administrator sign-in dialog with browser autofill and password-manager support.
    • Added server-side token verification, clearer validation errors, cancellation handling, and secure in-memory token use.
    • Added administrator authentication translations across supported languages.
    • Streamlined locale handling for authentication prompts.
  • Documentation

    • Documented local and remote dashboard authentication, including browser-managed autofill and non-persistent token handling.
  • Tests

    • Added coverage for validation, retries, cancellation, cleanup, focus restoration, and token security.

@coderabbitai

coderabbitai Bot commented Aug 2, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The dashboard now uses a localized password form for admin authentication. The API verifies submitted tokens through /api/settings and uses an injectable prompt callback after unauthorized responses. Tokens remain out of browser storage. Tests cover submission, cancellation, cleanup, focus restoration, locale handling, and API retries.

Changes

Admin token authentication

Layer / File(s) Summary
Token dialog and localized form
gui/src/admin-token-dialog.ts, gui/src/i18n/*, gui/tests/admin-token-dialog.test.ts
Adds a localized modal with a fixed read-only username and autofill-compatible token field. The dialog trims tokens, rejects empty input, handles rejected and unavailable states, restores focus, removes itself, and does not write to browser storage.
API verification and prompt injection
gui/src/api.ts, gui/tests/api-auth-memory.test.ts
Validates tokens through /api/settings, replaces window.prompt with an injectable callback, limits prompting per request wave, and restores custom prompt callbacks during test reset. Tests verify invalid-token retry behavior and empty or canceled input handling.
Dashboard authentication documentation
docs-site/src/content/docs/guides/web-dashboard.md, docs-site/src/content/docs/ja/guides/web-dashboard.md, docs-site/src/content/docs/ko/guides/web-dashboard.md, docs-site/src/content/docs/ru/guides/web-dashboard.md, docs-site/src/content/docs/zh-cn/guides/web-dashboard.md
Documents loopback sessions, remote admin-token requirements, password-manager support, and in-memory token handling.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ApiAuthFetch
  participant requestAdminToken
  participant verifyAdminToken
  participant promptForAdminToken
  participant BrowserForm
  ApiAuthFetch->>requestAdminToken: receive unauthorized response
  requestAdminToken->>promptForAdminToken: request admin token
  promptForAdminToken->>BrowserForm: show localized password form
  BrowserForm-->>promptForAdminToken: submit token or cancel
  promptForAdminToken->>verifyAdminToken: verify submitted token
  verifyAdminToken-->>promptForAdminToken: accepted, rejected, or unavailable
  promptForAdminToken-->>requestAdminToken: return token or null
  requestAdminToken-->>ApiAuthFetch: retry authenticated request
Loading

Possibly related PRs

Suggested labels: enhancement, documentation

Suggested reviewers: ingwannu, wibias, lidge-jun

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 36.36% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the primary change: enabling password-manager autofill for the admin-token sign-in flow.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@mincia1110
mincia1110 marked this pull request as ready for review August 2, 2026 02:16

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d1ee3644b8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread gui/src/admin-token-dialog.ts Outdated
Comment thread gui/src/admin-token-dialog.ts Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs-site/src/content/docs/guides/web-dashboard.md`:
- Around line 32-36: Update the translated dashboard guide pages for ja, ko, ru,
and zh-cn by adding the missing sign-in section immediately after their opening
instructions. Mirror the source guide’s behavior: explain the loopback
exception, require an admin token for remote dashboards, describe the standard
password form and browser/password-manager autofill, and state that the
dashboard retains the token only in memory without localStorage or
sessionStorage persistence.

In `@gui/src/admin-token-dialog.ts`:
- Around line 39-69: Replace the duplicated field-label text in the admin token
dialog with purpose-specific i18n entries: add account and admin-token
field-label keys to all six locale dictionaries, then use the localized values
for accountLabel.textContent and tokenLabel.textContent. Keep
ADMIN_TOKEN_USERNAME only as the readonly username value, and do not reuse
titleText for the token label or hardcoded text for user-visible labels.

In `@gui/tests/admin-token-dialog.test.ts`:
- Around line 56-67: Add a test alongside the existing admin-token dialog tests
that submits an empty or whitespace-only password, verifies the dialog remains
open and the prompt promise is still pending, then submits a valid token and
asserts the promise resolves successfully. Reuse the existing
promptForAdminToken dialog setup and submission interaction symbols.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 708924f4-06db-4671-9dd0-801701333b8e

📥 Commits

Reviewing files that changed from the base of the PR and between 6ac6bea and d1ee364.

📒 Files selected for processing (11)
  • docs-site/src/content/docs/guides/web-dashboard.md
  • gui/src/admin-token-dialog.ts
  • gui/src/api.ts
  • gui/src/i18n/de.ts
  • gui/src/i18n/en.ts
  • gui/src/i18n/ja.ts
  • gui/src/i18n/ko.ts
  • gui/src/i18n/ru.ts
  • gui/src/i18n/zh.ts
  • gui/tests/admin-token-dialog.test.ts
  • gui/tests/api-auth-memory.test.ts

Comment thread docs-site/src/content/docs/guides/web-dashboard.md
Comment thread gui/src/admin-token-dialog.ts
Comment thread gui/tests/admin-token-dialog.test.ts
@Wibias

Wibias commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

[shipping-github] Verdict: approve-comment

PR: #873 - Enable password manager autofill for admin tokens
Head: 2411b18a on dev (mergeStateStatus: CLEAN)

Semantic propagation

  • Concepts audited: admin-token entry flow (prompt primitive), management-auth verification, active-locale handoff, dashboard docs contract
  • Authoritative sources: src/server/management-auth.ts requireManagementAuth (401/503 semantics), src/server/auth-cors.ts (token source), gui/src/i18n/en.ts (TKey source of truth)
  • Producers and consumers checked: gui/src/api.ts 401 gate -> promptForAdminToken -> verifyAdminToken (/api/settings) -> storeToken/retry; CLI header consumers (cli/opencode.ts, cli/login-cli.ts) unaffected
  • Public/derived representations checked: X-OpenCodex-API-Key header contract; docs-site web-dashboard.md (en/ja/ko/ru/zh-cn); reference/management-api.md still accurate (server unchanged)
  • Material variant partitions checked: loopback GUI session vs raw admin token; 401 vs 503 vs network failure; six GUI locales; environment vs file token source
  • Positive and negative assertions checked: accepted/rejected/whitespace/cancel/locale dialog tests; memory-only, cross-origin and concurrent-401 API tests; privacy scan
  • Unmapped surfaces: none
  • Unproven equivalence assumptions: none
  • Representation mismatches: none
  • Variant coverage gaps: the dialog unavailable branch (network error / 503) has no direct unit test; branch is trivial and fail-safe (keeps the form open). Non-blocking
  • Axis verdict: pass

Linked: none

Usefulness

Real UX gap: window.prompt() exposes no username/password fields or autocomplete metadata, so password managers cannot save or autofill the admin token on remote dashboards. The new dialog (gui/src/admin-token-dialog.ts) exposes stable username + current-password fields with autocomplete attributes, and the earlier Codex review feedback (keep the form open until verified; use the active UI locale) is implemented in 2411b18a.

Bugs / correctness

  • Method: bug-review.md - Bugbot: n/a (Codex host) - complementary lenses: done (silent_failures, resource_leaks, edge_cases, error propagation, cancellation, races)
  • Findings: none blocking. Residual Low: the verification request has no timeout/abort; a hung /api/settings keeps the submit disabled until the user cancels (cancel works and restores focus). Not a regression versus the old prompt
  • Fixed this session: none (review-only run)

Security

  • Scope reviewed: authn, authz, secrets/config, SSRF/outbound, removed controls (scope: full)
  • Findings: none confirmed. The token stays module-memory only (memoryToken); the dialog builds DOM via textContent (no HTML injection); verification is a same-origin authenticated GET with cache: no-store; a 401 keeps the form open (no fail-open); no new storage writes; bun run privacy:scan green
  • Fixed this session: none

Spec / standards

  • Spec source: PR body (no linked issue). Claimed items all implemented: sign-in form, memory-only token, shared 401 gate, cancellation + focus restoration, localized copy (all six locales), regression tests, docs in all five doc locales
  • Standards: gui/AGENTS.md i18n rules satisfied (visible strings via DICTS; ADMIN_TOKEN_USERNAME is an allowed product-name literal); bun run lint:i18n green; no changelog practice in this repo

Reviews

  • Owners/maintainers: no open human threads
  • Bots (CodeRabbit / Codex): 5/5 threads resolved or acknowledged in 2411b18a (keep-open + token validation, active-locale handoff, localized labels, whitespace test, translated docs); 0 unresolved

Base / CI

  • Behind/conflicts: clean (mergeStateStatus: CLEAN, head 2411b18a)
  • Required checks: green on head - ubuntu, windows (17m32s), macos, npm-global x3, react-doctor, enforce-target, label, CodeRabbit
  • Local tip compile/tests: bun x tsc --noEmit pass; cd gui && bun test tests 492 pass; GUI lint, lint:i18n, GUI build, docs-site build, privacy:scan all pass; the full bun test --isolate tests suite is green in the CI matrix

Gate

none (not draft, no WIP/do-not-merge)

Bottom line

Useful and clean: the dialog replaces the prompt primitive with a standard sign-in form while preserving the shared 401 gate and memory-only token handling; all bot feedback is addressed, required CI is green on the reviewed head, and the authoritative ship gate returns ready. Verdict only - no approval or merge performed by this run.

@Wibias

Wibias commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

Thanks @mincia1110 - merging this.

Why it helps: remote dashboards can now sign in with a real password form instead of window.prompt(), so browser password managers can save and autofill the admin token. The dialog verifies the token with a safe read before closing, keeps it memory-only, and the sign-in flow is documented in every dashboard guide locale.

@Wibias
Wibias merged commit 56b4181 into lidge-jun:dev Aug 3, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants